I initially wanted to have 1 class per hook replacement for better readability and maintenance but it will break the OS rule about not extra naming what we should follow. If I have 1 class per hook replacement the className would have the same name as the method (kind of).
IE: replacement of social_post_form_post_form_alter
ClassName SocialPostFormPostFormAlterHook, method called SocialPostFormPostFormAlterHook.
I'm really up for suggestions here if we group hooks together or not in a class? (my preference is not) And then how do we name the class and the method?

My first instinct would be to split it in different classes. There will likely be cases where we it makes more sense to group them, e.g. when multiple hooks execute the same underlying code.

And I would ignore the rule about extra naming. Maybe it looks a bit weird, but while reading it you know what it does 🤷

If I look at what the hook does in this case then it's preparing the post form to be personalised to the user and to the context?

What about Drupal\social_post\Hooks\PostFormPersonalisation as fully qualified name? It indicates it's doing something to the Post form and it's personalising. "Social" is omitted because that's included in the namespace already which IDEs also include in their class search.

My first instinct would be to split it in different classes. There will likely be cases where we it makes more sense to group them, e.g. when multiple hooks execute the same underlying code.

And I would ignore the rule about extra naming. Maybe it looks a bit weird, but while reading it you know what it does 🤷

Coding guideline 3.6 Use service autowiring and alias interfaces to concrete ID'd service definitions suggests some changes here.

Preferably the service itself would be an interface that would be used for type-hinting, but I don't think that makes sense here.

(Drupal Core sets the alias the other way around but I don't really understand why, since Symfony's preferrable way is to use interfaces rather than distinct names).

We can create an Interface for every things we create but in that case I dont see the point. If i s required I'm happy to comply otherwise i think it s unnecessary artefact.

I agreed in my message that an interface may not be necessary here. We should still make sure that we use autowiring and define our services in a way so that this class can be used by another service that uses autowiring. That's what my suggested code snippet achieves.

Fixed

Accessing the context system in this way was new to me and I was wondering whether we should be calling into the group service directly like this.

While figuring out how the system works I came about the context.repository system which we can also use by providing the service name and context (@group.group_route_context:group) to its getRuntimeContexts methods. The benefit of using that system is that it will also statically cache the context value so it's not recomputed in case the method is called multiple times.

What do you think of moving this over to Drupal's context repository rather than calling a system directly? It also means that if we want to conditionally pull from multiple contexts in the future we don't need another service.

tbh i was looking at how to, and the only way that was working was that.
I can try your approach.

I think the context class provides you with a shorter way to do this that doesn't require traversing the data itself, by calling getContextValue directly on the context. Since that throws an exception if a required context isn't present (and group is required) we should check for hasContextValue first though.

Since PHPStan likely can't entirely follow the types we can add an assert to help show the invariant.

There's one stylistic thing I disagree with and one functional issue in the code:

Stylistically: I'm unsure why the === TRUE is added here since that's the default for if-statements. Especially since is_int returns an exact boolean and no truthy values. if (is_int($group)) is less complex to view and equivalent so it should be preferred.

Functionally: This code will never be true. The group route service resolves the context and returns \Drupal\group\Entity\GroupInterface|null as type for the context service.

In the old code we would get the parameter from the route directly. That can be an integer in the case of views (which doesn't configure upcasting for its arguments). However, the group implementation converts that to NULL due to the following line of code in GroupRouteContextTrait::getGroupFromRoute: if (($group = $route_match->getParameter('group')) && $group instanceof GroupInterface) {.

Fixing that on our side is slightly more difficult because it means that things that are used in views (Actions/Blocks/Forms) should use the plugin context system or the form context system rather than getting the context from the route. That way they get the instance of the context that Views itself has converted into an entity. So basically anything that might need this service for this code-path, shouldn't use this service.

I think if $form_object is not a ContentEntityForm then we have an implementation error right? So we probably want to make this an assert if we expect to catch this in testing or throw an actual exception if we think this might happen at run-time but shows an implementation error. The alternative would likely be a subtle and hard to debug bug (the post form not being altered) due to something that's wrong in an unrelated system (the post form no longer being a content entity form).

This if-statement also causes $content to be calculated but never used, so that statement should likely be after this guard clause.

$content is only ever used here so I would suggest inlining the variable. That has the advantage that if this is a post being edited we don't need to calculate $content.

I wonder if we can re-arrange this logic to make sure we don't set the default value and then overwrite it but only set the default value if we haven't picked another value. In this case we know setFormTitleAndPlaceholder is a very light-weight function, but since it's abstracted, we really shouldn't need to know this and the function might be calculating pi or fetching expensive data from the database unnecessarily. We should not be afraid to use else if it makes sense :)

the proper auto-wiring should looks like this (credit Alexander)

#hooks replacement
# use autowiring for arguments for `SocialPostFormHooks`
social_post.form.hooks:
  class: Drupal\social_post\Hooks\SocialPostFormHooks
  autowire: true
  tags:
  - { name: hooks }
# make `SocialPostFormHooks` injectable in another service (e.g. for decoration)
Drupal\social_post\Hooks\SocialPostFormHooks: '@social_post.form.hooks'